standalone doenetml in iframes can initialize in sequence#880
Open
dqnykamp wants to merge 9 commits intoDoenet:mainfrom
Open
standalone doenetml in iframes can initialize in sequence#880dqnykamp wants to merge 9 commits intoDoenet:mainfrom
dqnykamp wants to merge 9 commits intoDoenet:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a coordination mechanism for standalone DoenetML documents embedded in iframes to address memory and performance issues when multiple activities initialize simultaneously. The solution adds parent-child iframe coordination with two strategies: dom-order (sequential initialization) and viewport-first (prioritize visible iframes).
Changes:
- Added
initializeDoenetParentCoordinator()function to manage serialized iframe initialization with configurable strategies - Enhanced
renderDoenetViewerToContainer()to supportenableParentCoordinationflag and message-based coordination with parent window - Implemented comprehensive test suite with Cypress tests covering uncoordinated and coordinated scenarios
- Added extensive documentation in README.md and new COORDINATION.md explaining usage patterns and implementation details
Reviewed changes
Copilot reviewed 22 out of 22 changed files in this pull request and generated 22 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/standalone/src/index.tsx | Core implementation of parent-child coordination using postMessage protocol, IntersectionObserver for visibility tracking, and strategy-based iframe initialization |
| packages/standalone/README.md | Updated with quick start examples, API reference, and coordination configuration |
| packages/standalone/COORDINATION.md | New comprehensive documentation covering coordination strategies, timing, edge cases, and implementation details |
| packages/standalone/iframe-parent.html | Demo page showing dom-order coordination strategy |
| packages/standalone/iframe-parent-viewport-first.html | Demo page showing viewport-first coordination strategy |
| packages/standalone/iframe-child.html | Child iframe demo with parent coordination enabled |
| packages/standalone/index.html | Cleaned up test attributes from viewer containers |
| packages/test-cypress/vite.config.ts | Added standalone dist files to static copy for Cypress tests |
| packages/test-cypress/package.json | Added standalone package as build dependency and public files to watch list |
| packages/test-cypress/public/standaloneBlobUrls.js | Utility module for loading standalone scripts/CSS as blob URLs in tests |
| packages/test-cypress/public/uncoordinated-same-page.html | Test page with multiple viewers on same page without coordination |
| packages/test-cypress/public/uncoordinated-iframes.html | Test page with multiple uncoordinated iframes |
| packages/test-cypress/public/uncoordinated-child.html | Child page for uncoordinated iframe tests |
| packages/test-cypress/public/coordination-dom-order.html | Test page demonstrating dom-order coordination strategy |
| packages/test-cypress/public/coordination-viewport-first.html | Test page demonstrating viewport-first coordination with scroll scenarios |
| packages/test-cypress/public/coordination-child.html | Child page with coordination enabled for tests |
| packages/test-cypress/cypress/support/commands.js | Added getIframeBody helper command for testing iframe content |
| packages/test-cypress/cypress/e2e/standalone/uncoordinated-same-page.cy.js | Tests for multiple viewers on same page |
| packages/test-cypress/cypress/e2e/standalone/uncoordinated-iframes.cy.js | Tests for uncoordinated iframe initialization |
| packages/test-cypress/cypress/e2e/standalone/coordination-dom-order.cy.js | Tests verifying strict DOM order initialization |
| packages/test-cypress/cypress/e2e/standalone/coordination-viewport-first.cy.js | Tests verifying viewport-first prioritization from top and bottom |
| packages/doenetml/src/index.ts | Exported DoenetMLFlags type for better TypeScript support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
siefkenj
reviewed
Feb 26, 2026
|
|
||
| const viewerRoots = new WeakMap<Element, ReactDOM.Root>(); | ||
|
|
||
| type ViewerConfig = Record<string, any>; |
Contributor
There was a problem hiding this comment.
I'd be happier if this were an actual type with appropriate keys.
| ...flags | ||
| } = attrs; | ||
|
|
||
| const localConfig = { ...(config ?? {}) }; |
Contributor
There was a problem hiding this comment.
Use a default argument instead of ?? {}
| const userOnInit = localConfig.onInit; | ||
| delete localConfig.flags; | ||
| delete localConfig.initializedCallback; | ||
| delete localConfig.onInit; |
Contributor
There was a problem hiding this comment.
deleteing things from an object smells like an anti-pattern
| const visibilityRootMargin = | ||
| localConfig.visibilityRootMargin ?? | ||
| visibilityRootMarginFromAttrs ?? | ||
| "600px"; |
Contributor
There was a problem hiding this comment.
How much configuration does this all need? Could this be served by a top-level const instead? Is it going to vary from situation to situation?
| const shouldCoordinate = | ||
| localConfig.enableParentCoordination ?? | ||
| enableParentCoordination ?? | ||
| false; |
Contributor
There was a problem hiding this comment.
Do we want the default to be false? I'm pretty unhappy with the way this code is layed out. It makes defaults very hard to find without reading the whole file.
|
|
||
| // Check if we can access parent (same origin) | ||
| try { | ||
| void window.parent.location.origin; |
Contributor
There was a problem hiding this comment.
why void? Won't this test work without this?
| } | ||
|
|
||
| // Set up coordination with parent | ||
| const iframeId = `iframe_${Date.now()}_${Math.random().toString(36).slice(2, 9)}`; |
Contributor
There was a problem hiding this comment.
Make a uniqueId function if you're going to use one multiple times (I believe you already have one somewhere in the code for all your sending back and forth of scores and whatnot)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR adds two coordination modes to standalone DoenetML documents embedded in iframes:
dom-orderandviewport-only.Without coordination (the default), all DoenetML activities initialize at once. This behavior can lead to performance and memory issues in a page that contains many DoenetML activities.
To force the DoenetML activities to initialize in sequence, we've added an
enableParentCoordinationflag that can be added when embedding a DoenetML activity. If this flag as specified, the DoenetML will wait for a signal from its iframe parent before initializing.The coordination is enabled and the activity intialization is unlocked by calling a new function
initializeDoenetParentCoordinator()in the parent of the iframes. It will coordinate the initialization to occur either in the order that elements appear in the DOM (default "dom-order" strategy) or to prioritize activities that are currently visible (the "viewport-first" strategy).The PR also updates documentation about the coordination and updates the README.
Resolves #874.